Skip to content

Conversation

@litherum
Copy link
Contributor

@litherum litherum commented Oct 1, 2022

This patch does a few things:

  1. Fixes a small handful of whitespace / typo problems
  2. Adds an example showing how the font-weight descriptor can accept a range
  3. Advises authors to use high-level font-variant properties instead of font-feature-settings
  4. Rewrites an example to follow the above advice
  5. Advises authors to use high-level font properties instead of font-variation-settings

This patch isn't quite ready to land; there's a problem where a <ol> and a <table> inside a <p class="advisement"> do not actually end up in the advisement box in the rendered content. @tabatkins do you know how to fix this?

cc (@PeterConstable) via https://twitter.com/pgconstable/status/1575526680076357633

…ngs and font-variation-settings

This patch does a few things:
1. Fixes a small handful of whitespace / typo problems
2. Adds an example showing how the font-weight descriptor can accept a range
3. Advises authors to use high-level font-variant properties instead of font-feature-settings
4. Rewrites an example to follow the above advice
5. Advises authors to use high-level font properties instead of font-variation-settings
@litherum litherum requested review from drott and svgeesus October 1, 2022 05:38
@RoelN
Copy link

RoelN commented Oct 1, 2022

Great addition, thanks for the clear examples!

(Super nitty nitpick, if best practices are to be used in the examples, make the WOFF font a WOFF2 font?)

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice improvement.

(I suggested a change that fixes the markup issue noted)

@litherum litherum merged commit b1e23c2 into w3c:main Oct 3, 2022
@litherum litherum deleted the fontadvice branch October 3, 2022 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants